Fix docstring issue and a broken link #1576
Conversation
| @@ -186,6 +186,6 @@ which is used for serving `reveal.js`_ HTML slideshows. | |||
| .. links: | |||
|
|
|||
| .. _jinja: http://jinja.pocoo.org/ | |||
| .. _filter: http://jinja.pocoo.org/docs/dev/templates/#filters | |||
There was a problem hiding this comment.
Hmm still seems to be complaining about pocoo.org fetches -- maybe the line before or somewhere else in the project we're fetching objects that don't exist anymore?
There was a problem hiding this comment.
yes @MSeal ! i'm really confused about the CI issue. Here is what i know.
that url - jinja.pocoo.org/docs resolves to https://jinja.palletsprojects.com/en/3.0.x/
i've definitely seen builds have a hard time with redirects. i just fixed that but i'm still seeing that CI issue. i suspect somewhere there are other pocoo.org references perhaps? i can search for that next.
http://jinja.pocoo.org/docs/dev/objects.inv
i think i fixed it. the link for intersphinx_mapping was the old url. give me a few minutes to push it up. fingers crossed :)
There was a problem hiding this comment.
fixed this URL to be the new jinja docs link. palletsprojects.com
docs/environment.yml
Outdated
| @@ -12,5 +12,6 @@ dependencies: | |||
| - tornado | |||
| - entrypoints | |||
| - ipykernel | |||
| - pip | |||
There was a problem hiding this comment.
this was merged in my last pr. speeds up the build and simplifies the uutput
| @@ -151,7 +151,7 @@ Configurable classes. | |||
| .. seealso:: | |||
|
|
|||
| - :doc:`customizing` | |||
| - :ref:`jinja:filters` | |||
| - `More on Jinja Filters`_ | |||
There was a problem hiding this comment.
updated link. this used :ref: before which is a local page link. I also fixed the URL to the jinja template page
docs/source/conf.py
Outdated
| @@ -321,6 +321,6 @@ | |||
| # Example configuration for intersphinx: refer to the Python standard library. | |||
| intersphinx_mapping = { | |||
| 'python': ('https://docs.python.org/3.6', None), | |||
| 'jinja': ('http://jinja.pocoo.org/docs/dev', None), | |||
| 'jinja': ('https://jinja.palletsprojects.com/en/latest/', None), | |||
There was a problem hiding this comment.
I suspect this was failing because the URL was a redirect and builds don't like redirects. maybe it was just a front end redirect which can cause problems. the url above is correct.
| capable of passing additional information to the Jinja | ||
| templating engine. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| preprocessor : `Preprocessor` | ||
| preprocessor : `nbconvert.preprocessors.Preprocessor` |
There was a problem hiding this comment.
i am not sure if this is the correct preprocessor object. It is either this or .base.Preprocessor. but this was causing the docs to fail too.
| @@ -316,7 +316,7 @@ def _preprocess(self, nb, resources): | |||
| # to each preprocessor | |||
| for preprocessor in self._preprocessors: | |||
| nbc, resc = preprocessor(nbc, resc) | |||
| try: | |||
| try: | |||
There was a problem hiding this comment.
my text editor Atom must do something with spaces. i hope this isn't problematic. if it is i can try to fix it.
|
ok the new point of failure is similar to the original issue i fixed. this is a simple docstring fix if we can identify which exporter object to use Which one is the right one? NOTE i don't get this error building locally! |
|
ok i may need input from you @MSeal or @yuvipanda this seems to be a circular problem. once i fix a reference in a doc i get a new one. The nature of each issue is a potentially duplicative reference to an object that has a base object with the same name and then maybe on that inherits or something - like the warning (treated as an error) below. i'm not familiar with the extension that is testing your docstrings for objects. and each time i've guessed which object is the correct one. the base one or the inherited one. i'm open to any input and also open to undoing anything ive done so far. i am confident that the jinja fixes are good. but i cant replicate the other issues (like the one pasted below here) locally to troubleshoot.
|
|
Sorry it took a while for me to get back to this thread. I was able to get the builds working with all but the fitlers.rst |
|
Can't seem to get the errors to occur locally, so I have to keep pushing fixes :/ |
|
@MSeal I had the same issue - i think the errors are related to this sphinx issue but i noticed the docs are not updating so my guess i s until this is fixed they wont build on readthedocs |
|
one thing i find odd is it's flagging files that are just code not even references in docstrings. i stopped when i noticed that because i wasn't sure if/how we wanted to fix that! it seems like sphinx should ignore that type of issue or have an option to ignore it. |
|
Yeah, we have all warnings as errors causing the build annoyance. Most of the references are in the changelog, but for now until that reference issue is resolved in sphinx I'm just going to change them to non-references for now to unblock. |
|
Uhg, 'CellExecutionError' is being pulled in from nbclient which is outside the repo's ability to directly control |
|
My local sphinx was on 3.x, upgrading to 4.x explicitly gets the errors to occur. I fixed all the ones not brought in but didn't know a way to get around the imported reference issues so I turned off warnings as errors for now |
|
oh wow - docs built?! i'll check read the docs !! this is great @MSeal thanks! what a pain to fix all of those references! |
|
yay! docs updated finally!! 🎆 |
|
Yeah! I ended up having to disable warnings as errors until sphinx-doc/sphinx#4961 is resolved as the remaining references are coming from nbclient can't be eliminated locally to nbconvert. |
|
oh... good call. Yes that is really an annoying bug in sphinx right now! glad you were able to figure out how to just turn that off for the time being! |
When building the docs a few issues come up. One is a bad reference link that no longer exists on the page. Modified the link to be a jinja docs link for templates that works. The other is a docstring issue. will watch CI on this one and check back.